Bringing windows platform more in line with more complete win32 impl #3744
Conversation
|
@rurikoaraki and @PPatBoyd - any concerns here? Will review as well but this one should have a couple sets of eyes. |
Will make time to review today |
| @@ -33,7 +33,6 @@ export const defaultToggleButtonColorTokens: TokenSettings<ToggleButtonTokens, T | |||
| const highContrastColors = { | |||
| checked: { | |||
| backgroundColor: PlatformColor('SystemColorHighlightColor'), | |||
| borderColor: PlatformColor('SystemColorHighlightColor'), | |||
There was a problem hiding this comment.
Was this change to the border intended? #Resolved
There was a problem hiding this comment.
I see, it's to align with win32
| @@ -76,7 +76,8 @@ export const useButton = (props: ButtonProps): ButtonInfo => { | |||
| const hasTogglePattern = props.accessibilityActions && !!props.accessibilityActions.find((action) => action.name === 'Toggle'); | |||
|
|
|||
| const theme = useFluentTheme(); | |||
| const shouldUseTwoToneFocusBorder = Platform.OS === ('win32' as any) && props.appearance === 'primary' && !isHighContrast(theme); | |||
| const shouldUseTwoToneFocusBorder = | |||
There was a problem hiding this comment.
This also applies to the focusInnerBorder component on Button, ToggleButton
|
|
||
| export const defaultCompoundButtonTokens: TokenSettings<CompoundButtonTokens, Theme> = (theme: Theme): CompoundButtonTokens => ({ | ||
| medium: { | ||
| padding: globalTokens.size120 - globalTokens.stroke.width10, |
There was a problem hiding this comment.
I wonder if this will be needed on windows... we end up having to subtract the width of the border of the button to figure out the padding on win32 because when Views are bound by minHeight/Width instead of just height/width, the border adds to the height of the button so it ends up being bigger than intended (and sometimes causing size changes on focus). This (seemingly?) doesn't happen on web, so I wonder if windows has this problem or not
There was a problem hiding this comment.
(and we need minHeight for handling text scaling, and minWidth for just who knows how long strings are)
This change introduces our first .officewin file. The idea of the .officewin extension is to allow us to continue to use our theme fragments that are picked up by the bundler, but allow two themes for the windows platform. By default the windows platform should provide a windows fluent theme. But for office we want the default theme to have various officeisms. We will do that by having custom resolution logic when building against the windows platform within office. (Office will resolve *.officewin, *.windows, *.native for FURN packages when building against the windows platform.
Currently the only difference between the Windows theme and Office's theme for the .windows platform is that we override the default button size to be small within Office. But I expect that to expand as we dig into the details of the theming differences between Office and Windows.
Mostly this change unifies some theming of Button/Checkbox by sharing definitions of various parts of the theme between win32 and windows (by moving the win32 implementation to windows). The win32 definitions are more complete and more tested than the current windows ones. Over time as we work out that parts of those themes are more Office specific, we could move those parts of the definitions to .officewin files.